Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adding adapters for MySQL and Postgres #37

Merged
merged 1 commit into from
Apr 7, 2016
Merged

Adding adapters for MySQL and Postgres #37

merged 1 commit into from
Apr 7, 2016

Conversation

volkanunsal
Copy link
Contributor

No description provided.

@volkanunsal volkanunsal mentioned this pull request Mar 23, 2016
Closed
nettofarah
nettofarah reviewed Mar 24, 2016
View reviewed changes
lib/polo/adapters/postgres.rb
module Adapters
module Postgres
def on_duplicate_key_update(inserts, records)
raise 'Not implemented for Postgres'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should be a little more specific here.
Let's change it to: on_duplicate: :override is not currently supported in the PostgreSQL adapter

@nettofarah
Copy link
Contributor

@volkanunsal awesome PR!
I left some comments just to make sure we can keep the code consistent with the rest of the codebase.

Please make sure to squash all commits before and write a commit message in this format: http://chris.beams.io/posts/git-commit/

We should be good to merge after you're done making the changes and squashing the commits.

@volkanunsal
Copy link
Contributor Author

@nettofarah Thanks for the awesome line notes. I'm going to go through them later today. 👍

@volkanunsal
Copy link
Contributor Author

Sorry for the delay, but took care of most of the items on the list. Take a look.

@nettofarah
Copy link
Contributor

This looks awesome!
I'll take an extra look tomorrow, but everything looks good to me.

Congrats on your first PR to Polo :)

@volkanunsal
Copy link
Contributor Author

Ping!

@nettofarah nettofarah merged commit de90964 into IFTTT:master Apr 7, 2016
@nettofarah
Copy link
Contributor

Yay! Sorry for the delay.
I'll draft a release soon :)

@nettofarah
Copy link
Contributor

hey, @volkanunsal. would you mind writing a comment here? #19

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@volkanunsal @nettofarah